-
Notifications
You must be signed in to change notification settings - Fork 0
Hw18 #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
IvanKozlov98
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Параллельный лес: 25/25
отсутствие информативного readme -5б
В jupyter-ноутбуке многовато лишних импортов в месте RandomForestClassifierCustom
Не везде есть docstring-и и аннотации к типам -3б
История коммитов — то с большой, то с маленькой буквы начинаются -0.25
+0.25 за хорошие тесты :)
Постарайтесь сделать красивое и информативное readme -- хотя бы для себя, пока помните, где что лежит и как что-то делает. Чтобы в будущем было приятно возвращаться к этим работам :)
Итого: 42 балла
|
|
||
| def fit(self, X, y, n_processes=1): | ||
| self.classes_ = sorted(np.unique(y)) | ||
| def fit_tree_process(i, queue): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
почему бы эту функцию не сделать внутренней для класса?
внутренние функции стоит объявлять в начале функции и хотя бы строчку пропускать после определения
| queue = multiprocessing.Queue() | ||
| processes = [] | ||
| for i in range(self.n_estimators): | ||
| p = multiprocessing.Process(target=fit_tree_process, args=(i, queue)) | ||
| processes.append(p) | ||
| p.start() | ||
| results = [] | ||
| for _ in range(self.n_estimators): | ||
| results.append(queue.get()) | ||
| for p in processes: | ||
| p.join() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
чтобы не возитьтся с queue` можно было возвращать из функции собираемые данные
также, проще использовать готовые решения распределения задач -- например, map
однако ваше решение также пойдет :)
(особенно для тренировки)
| test_fasta_data = """>seq1 | ||
| ATGC | ||
| ATGC | ||
| ATGC | ||
| """ | ||
| expected_fasta_data = """>seq1 | ||
| ATGCATGCATGC | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
глобальные переменные даже в тестах не приветствуются :)
No description provided.